Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: listen on all interfaces for ecs-server #1199

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

frco9
Copy link

@frco9 frco9 commented Mar 22, 2023

This MR is a quick fix that should solve #1198

Currently ecs-server only listens to the loopback address making it unreachable from docker containers on Linux.

@mtibben
Copy link
Member

mtibben commented Mar 22, 2023

What are the potential security side-effects here? Should we be listening on all interfaces?

@frco9
Copy link
Author

frco9 commented Mar 22, 2023

That's a good question and it's a bit out of my domain of expertise. It would mean that if a user does not have a proper firewall, any user knowing the ip of the user, the port of the ecs-server and the auth token, would be able to request sessions token from the user's ecs-server.
I agree it's not perfect as it increase attack surface, although as the port and auth token is supposed to only be accessible from the aws-vault subprocess, it seems not that obvious to find them ?

Otherwise, this could be made configurable as initially proposed in the issue ? Don't you have this access denied issue on Linux ?

@mtibben
Copy link
Member

mtibben commented Mar 22, 2023

No, I use macOS. Perhaps we add a --listen-address flag that defaults to 127.0.0.1?

@frco9 frco9 force-pushed the fix/ecs-server-listen-on-all-interfaces branch from a325d69 to 8becf0d Compare March 22, 2023 13:54
@frco9
Copy link
Author

frco9 commented Mar 22, 2023

@mtibben, I've made the changes, I'm not really used to develop in Golang. I've successfully compile and test it locally but please tell me if things can be refactored or if I've missed something.

@frco9
Copy link
Author

frco9 commented Mar 24, 2023

@mtibben could you take a look ?

@mtibben
Copy link
Member

mtibben commented Mar 26, 2023

Tried this out, however realised that the aws cli (not sure about the sdk) doesn't actually allow arbitrary hosts

Unsupported host '::'.  Can only retrieve metadata from these hosts: 169.254.170.2, localhost, 127.0.0.1

So the problem seems confined to the docker scenario in the issue. Can you see if the solution described at #1198 (comment) works?

@frco9
Copy link
Author

frco9 commented Mar 27, 2023

Issue is just that BaseURL is not returning a valid https url that could be fetched it returns http://[::]:{port}.

This seems linked to golang/go#9334. To "fix" that and force to bind on ipv4 I have changed the network type from tcp to tcp4

Now base url returns the proper value:

AWS_CONTAINER_CREDENTIALS_FULL_URI=http://127.0.0.1:{port}

@RemaniTinpo
Copy link

👋

Hi,

I'm having the same issue as a Linux user.
Any chance that this PR would be merged?

Thanks a lot, @frco9 and @mtibben

@awilkins
Copy link

awilkins commented Aug 2, 2023

Hey there!

As per my comment in the issue, on Linux, the safest value to pass to this argument would be the output of this

docker network inspect bridge | jq '.[0].IPAM.Config[0].Gateway'

In conjunction with adding this to your docker-compose.

    extra_hosts:
     - host.docker.internal:host-gateway

0.0.0.0 isn't terrible because the ECS server requires an auth token for access, which is passed down to the child process of aws-vault exec via the environment. Because this isn't available to a hypothetical adversary elsewhere in the network, in theory you're safe. In practice, someone might find an exploit or DoS attack for the ECS server implementation, so it's safer to only listen to interfaces you're expecting traffic from.

So the full command (with this option added) would be

aws-vault exec \
    --ecs-server \
    --listen-address $(docker network inspect bridge | jq '.[0].IPAM.Config[0].Gateway') \
    my-profile -- \
    docker compose up --build aws-vault-proxy

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants